fix(core): Decrease number of Sentry stack frames for messages from captureConsoleIntegration
#18096
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Lms24
requested review from
a team,
JPeer264 and
chargome
and removed request for
a team
Contributor
size-limit report 📦
|
Lms24
changed the title
captureConsoleIntegrationcaptureConsoleIntegration
Contributor
node-overhead report 🧳Note: This is a synthetic benchmark with a minimal express app and does not necessarily reflect the real-world performance impact in an application.
|
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Change
This PR creates a synthetic exception already within the captureConsole handler, so that we minimize the number of Sentry stack frames in the stack trace. It also adjusts the
Client::captureMessagemethod to favor an already providedsyntheticExceptionover the one it would create.Context
If users set
attachStackTrace: truein their SDK config, our SDK will add stack frames from an internally created error ("synthetic exception") to events that don't have an original stack trace (e.g.captureMessagecalls orcaptureException('string error')). When usingcaptureConsoleIntegrationinstrumentation to automatically capture messages from console logs, the synthetic exception would be created so deeply in the call stack that the actualconsole.*call frame would no longer be in the stack trace due to Node limiting the number of frames to 10 by default.Users can increase the limit, for example by setting
Error.stackTraceLimit = N, but ideally they don't have to.Alternatives considered
We discussed setting
Error.stackTraceLimitto something like 50 (the max number of frames we parse) within the SDK but decided against it due to memory and payload concerns. We can revisit if stack frame limit problems get reported again.closes #18094